Skip to content

Conversation

@andrewheard
Copy link
Contributor

Added a protocol named FIRAppCheckProtocol to the FirebaseAppCheckInterop SDK that duplicates the token fetching public APIs of FIRAppCheck. Updated FIRAppCheck to publicly conform to FIRAppCheckProtocol.

This will allow other SDKs to accept an instance of App Check through a parameter of type id<FIRAppCheckProtocol>, while not requiring a hard dependency on FirebaseAppCheck.

#no-changelog

@google-oss-bot
Copy link

google-oss-bot commented Aug 8, 2023

Coverage Report 1

Affected Products

  • FirebaseFirestore-iOS-FirebaseFirestore.framework

    Overall coverage changed from 88.14% (f3c7db7) to 88.10% (2c1f46c) by -0.04%.

    FilenameBase (f3c7db7)Merge (2c1f46c)Diff
    exception.cc84.21%23.68%-60.53%
    leveldb_key.cc98.14%98.82%+0.69%
    local_serializer.cc87.78%88.33%+0.56%
    ordered_code.cc93.90%94.39%+0.49%
    write_stream.cc91.55%94.37%+2.82%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/TIwPzNW7HJ.html

@github-actions
Copy link
Contributor

github-actions bot commented Aug 21, 2023

Apple API Diff Report

Commit: 2c1f46c
Last updated: Mon Sep 18 22:23 PDT 2023
View workflow logs & download artifacts


FirebaseAppCheck

Classes

FIRAppCheck
[REMOVED] -tokenForcingRefresh:completion:
Swift:
-  func token ( forcingRefresh : Bool ) async throws -> FIRAppCheckToken
Objective-C:
-  - ( void ) tokenForcingRefresh :( BOOL ) forcingRefresh completion :( nonnull void ( ^ )( FIRAppCheckToken * _Nullable , NSError * _Nullable )) handler ;
[REMOVED] -limitedUseTokenWithCompletion:
Swift:
-  func limitedUseToken () async throws -> FIRAppCheckToken
Objective-C:
-  - ( void ) limitedUseTokenWithCompletion : ( nonnull void ( ^ )( FIRAppCheckToken * _Nullable , NSError * _Nullable )) handler ;

@andrewheard andrewheard merged commit a471cc8 into master Sep 14, 2023
@andrewheard andrewheard deleted the ah/app-check-protocol branch September 14, 2023 21:11
@mikehardy
Copy link
Contributor

@paulb777 / @andrewheard

When I attempt to update react-native-firebase to firebase-ios-sdk 10.16.0, including this PR, I receive this:

❌ /Users/mike/work/invertase/react-native-firebase/tests/ios/build/Build/Products/Debug-iphonesimulator/FirebaseAppCheck/FirebaseAppCheck.framework/Headers/FIRAppCheck.h:19:1: use of '@import' when C++ modules are disabled, consider using -fmodules and -fcxx-modules
@import FirebaseAppCheckInterop;
^

I traced it to this PR when I noticed that header file changed in my pod cache between 10.15.0 and 10.16.0. Is this an expected outcome? (stated differently, is there something I need to change so this works, ideally that wouldn't be a breaking change for us since we're adopting a feature release here...)

It's coming from here:
https://github.com/invertase/react-native-firebase/blob/c9b695aa8fd75d5a1d070ecbb6bb9ac4e9ff062e/tests/ios/testing/AppDelegate.mm#L21
Which goes to here:
https://github.com/invertase/react-native-firebase/blob/c9b695aa8fd75d5a1d070ecbb6bb9ac4e9ff062e/packages/app-check/ios/RNFBAppCheck/RNFBAppCheckModule.h#L22
Which goes to here:
https://github.com/invertase/react-native-firebase/blob/c9b695aa8fd75d5a1d070ecbb6bb9ac4e9ff062e/packages/app-check/ios/RNFBAppCheck/RNFBAppCheckProviderFactory.h#L18

...and that file doesn't go through with our compiler settings 🤔

Is there anything that can be done here?

In issues past, I have attempted to provide build instructions for our test app over in react-native-firebase so reproductions are easy but I/we have observed you all may have difficulty since it requires a relatively up to date javascript / node environment etc etc and that's a time suck if you don't already have it running. Let me know if you want repro instructions or any other information though, happy to collaborate of course

@paulb777
Copy link
Member

paulb777 commented Oct 9, 2023

@mikehardy Thanks for the report. We had an internal report of the same Objective C++ breakage earlier today so we shouldn't need a repro.

We plan to fix for the next release and expect to have the details sorted out in the next few days. Feel free to open an issue for tracking.

Roughly, a workaround is something like replacing the @import FirebaseAppCheckInterop with file imports of the public headers of the module.

@mikehardy
Copy link
Contributor

Okay, I opened #11916 but noted right at the top that this was known and in progress, just for tracking

I don't think as a cocoapods consumer there is a clean way for us to work around it. Just as an exercise, I tried hacking with the cocoapods cache versions of the App Chck podspec to include the interop headers in source files, and then adjust the @import to be an include etc but couldn't get it working and that's not really well-behaved anyway as a release-able thing. So we'll wait for 10.17.0 or 10.16.1.

Thanks!

@firebase firebase locked and limited conversation to collaborators Oct 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants